Remove negative runoff #471
Conversation
Avoid modifying fields in place so that the ROF import fields are truly what came from ROF. So this removal of negative runoff will just appear in the fields mapped to OCN.
|
@jedwards4b I'm assigning you as a reviewer mainly so you can help with the timing of bringing this in, since it will change answers. I expect @mvertens to do the actual code review. |
|
(To be clear on my last comment: @jedwards4b , I welcome a review from you if you want, but since I have been talking with @mvertens about this, I figured I'd save your time and just get a code review from her.) |
|
A few specific questions / comments: (1) Is it a problem that this will change behavior by default for all configurations? We can change the default of the new namelist flag if needed to avoid these answer changes. (2) Based on some spot-checks from a one-year run of compset
|
I'm not sure if this is necessary, but it seems like it could be in some configurations, and it won't hurt to do this check.
|
I have run the aux_cmeps test suite on derecho with these changes; tests pass and are bit-for-bit. I think this will only change answers in B compsets (or other configurations with both active land/river and active ocean). I have updated the top-level comment accordingly. I just updated to the latest CMEPS that includes the new glc runoff fields and ran test |
There was a problem hiding this comment.
@billsacks - this looks great. Thanks for the careful implementation.
Separate the control of removing negative runoff for lnd vs glc, and set default to false for glc-derived runoff ### Description of changes This is a follow-up to #471 . At least for now, we want the default to be to remove negative runoff for lnd-derived runoff, but NOT for glc-derived runoff. This PR implements that change. Note that this changes behavior for the correction of glc-derived runoff, but keeps the behavior the same as before for lnd-derived runoff. ### Specific notes Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial): **Yes - changes answers substantially for runoff fields in some B compsets (or other configurations with both active land/river and active ocean) - but only for configurations that have glc-derived runoff; this can currently come from configurations with CISM in EVOLVE mode or DGLC (even in NOEVOLVE mode). Note that this change just undoes part of the answer changes that came from #471 .** Any User Interface Changes (namelist or namelist defaults changes)? Separates the namelist item remove_negative_runoff into two separate options: remove_negative_runoff_lnd and remove_negative_runoff_glc. ### Testing performed Please describe the tests along with the target model and machine(s) If possible, please also added hashes that were used in the testing Using `cesm2_3_beta17` (with CISM at `756cfa6f0514d89977d2732ad06a4d364da5d418`, mosart at mosart1.1.02, and removing the contents of `cime_config/testmods_dirs/allactive/defaultio/user_nl_mosart`: Ran three versions of `SMS_D_Ld3.f19_g17.B1850G.derecho_intel.allactive-cism-test_coupling`: - One with default flags - One with `remove_negative_runoff_glc` set to true - One with `remove_negative_runoff_lnd` set to false I checked the behavior in these in runs with dbug_flag set to 21 so I would see diagnostics of what fields were being adjusted, and confirmed that the correct fields were being operated on in all three cases.
Description of changes
Remove negative runoff before mapping rof -> ocn by setting negative runoff to 0 and downweighting all positive runoff appropriately so that the global runoff sum is the same as before.
Specific notes
Contributors other than yourself, if any: @mvertens
CMEPS Issues Fixed (include github issue #):
Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial) : Yes - changes answers substantially for runoff fields in B compsets (or other configurations with both active land/river and active ocean) (appears not to change answers in most / all other configurations)
Any User Interface Changes (namelist or namelist defaults changes)? New namelist item: remove_negative_runoff (true by default)
Testing performed
Tested in the context of cesm2_3_beta17
Ran a number of tests using compset
1850_CAM60_CLM50%BGC-CROP_CICE_POP2%ECO_MOSART_SGLC_WW3_SESP_BGC%BDRDand resf19_g17.Also ran aux_cmeps test suite on derecho; all tests passed and were bit-for-bit except for tests that also failed in the baseline:
I think the passing tests were bit-for-bit because they didn't include any B compset tests, and it appears that runoff is non-negative in the tests that use drof.
I also ran one nag test on izumi, which also passed and was bit-for-bit:
SMS_D_Ld5_P32.f10_f10_mg37.I2000Clm50BgcCropRtm.izumi_nag.rtm-default(I didn't run the full aux_cmeps test suite on izumi, because I noticed that most of the izumi aux_cmeps tests failed in the baseline).